Connecting timeout #2736
Merged
Connecting timeout #2736
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rossabaker
reviewed
Jul 22, 2019
| @@ -20,6 +23,7 @@ sealed abstract class BlazeClientBuilder[F[_]] private ( | |||
| val responseHeaderTimeout: Duration, | |||
| val idleTimeout: Duration, | |||
| val requestTimeout: Duration, | |||
| val connectingTimeout: Duration, | |||
There was a problem hiding this comment.
Bikeshed: async-http-client and jetty-client both call this connectTimeout. Maybe that's a better name for consistency. Would also affect the blaze PR.
| @@ -33,6 +37,7 @@ sealed abstract class BlazeClientBuilder[F[_]] private ( | |||
| val parserMode: ParserMode, | |||
| val bufferSize: Int, | |||
| val executionContext: ExecutionContext, | |||
| val scheduler: Option[TickWheelExecutor], | |||
There was a problem hiding this comment.
I think I like the Ember idiom for defaultable resources, where the builder provides a default, or an unmanaged one can be passed in.
|
The build fails due to binary compatibility issue in BlazeClientBuilder and Http1Supoport constructors. Both constructors are private. Do we really care about binary compatibility of private APIs? @rossabaker how should I approach this topic? |
|
It should be safe to add a MiMa exclusion to the build for both of those. |
…uilder and Http1Support
RafalSumislawski
force-pushed
the
connecting-timeout
branch
from
62d33d1
to
95f08e5
Compare
|
I think both test failures are flakes. Restarted. |
ChristopherDavenport
approved these changes
Jul 30, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow-up to http4s/blaze#308.
WIP: It won't compile until the aforementioned PR is merged and blaze 0.14.7 is released.
This PR:
connectingTimeoutfor configuring the newly introducedconnectingTimeoutin blaze'sClientChannelFactoryFixes: #2386, #2338, #2690